Pthreads support for Node.js #9745

kripken · 2019-10-31T00:30:02Z

This ended up tricky (see the dates on the commits...) because of various node/web differences, like the global scope, the event loop, APIs, etc. With help on #6567 I think this PR finally manages to make it work, at least for an initial "hello world" type test. Thanks to everyone on that issue and in particular @addaleax!

Followups should add a lot more tests. Sadly I don't think we can just reuse the existing browser tests, as they make some web assumptions (like reporting the result using an xhr, when it's ok to block and when not, etc.).

This moves some code from shell.js into side .js files, so that we can use it in multiple places.

Fixes #6567

addaleax

✨

src/worker.js

addaleax · 2019-10-31T00:35:45Z

src/worker.js

+  var nodeFS = require('fs');
+
+  var nodeRead = function(filename) {
+    return nodeFS.readFileSync(filename).toString();


Suggested change

return nodeFS.readFileSync(filename).toString();

return nodeFS.readFileSync(filename, 'utf8');

(Just a minor thing, it avoids creating an intermediate Buffer/copying it into a string)

Nice, thanks! I think it's best to open a separate PR for that later as I see we have more than one existing call like that that could be improved that way.

src/worker.js

src/library_pthread.js

src/compiler.js

src/runtime_init_memory.js

src/shell.js

dschuff · 2019-10-31T23:06:56Z

tests/core/pthread/create.cpp

@@ -0,0 +1,62 @@
+// Copyright 2019 The Emscripten Authors.  All rights reserved.


we have tests/pthread/test_pthread_create.cpp already. How does tests/core/pthread relate to the tests in tests/pthread?

tests/pthread/ are existing tests, that only work on the web. The new core/pthreads are for the core test suite and run on node.

Sadly the web tests can't just be reused as they make some web assumptions (like reporting the result using an xhr, when it's ok to block and when not, etc., which it turns out node differs on).

I see. We should probably document somewhere (even if it's just for our own testing purposes) exactly the differences between web-pthreads and node-pthreads environments, and it would be nice to eventually unify the tests. But that doesn't need to be in this PR.

dschuff · 2019-10-31T23:08:40Z

tests/core/pthread/create.cpp

+    CreateThread(i);
+  }
+
+  // Node.js requires the event loop to be reached for the worker to start up.


Does this mean that PTHREAD_POOL doesn't work for node?

Good point, it should. It didn't in earlier work here, but looks like the current PR got it to work! I'm adding a test for the pool allowing works to be synchronously available.

See #9745 (comment)

abram · 2020-01-31T20:47:25Z

Hi @kripken, thanks for your work on this! I'm running into an issue and I wanted to check in with you before working on a fix.

I'm currently deploying a multithreaded application using Emscripten inside Electron, and this change appears to have broken things for me - I get errors like worker.on is not a function. I think the problem stems from the Electron issue electron/electron#18540. Inside current Electron windows/workers (when the properties nodeIntegration/nodeIntegrationInWorker are set to true), most Node APIs are available, but the worker_threads API isn't. Since Emscripten attempts to use the Node API instead of the web one, setup fails.

The simplest solution I can think of is to switch the priorities, so web workers are preferred over node workers. Or additional checks could be added to confirm that the Node worker_threads API is available before choosing to use it.

What do you think? If it would be helpful I could put together a minimal test app demonstrating the problem.

kripken · 2020-01-31T23:08:27Z

@abram Interesting, I didn't realize how complex this could be I guess 😄 Are you saying we might check, in ENVIRONMENT_HAS_NODE whether we have Web (non-Node) Workers and use them if so? That sounds good to me if it works, PR would be welcome!

abram · 2020-02-01T16:15:02Z

@kripken Yes, that sounds right. I'll work on a PR. Thanks!

@addaleax

This ended up tricky because of various node/web differences, like the global scope, the event loop, APIs, etc. With help on emscripten-core#6567 I think this PR finally manages to make it work, at least for an initial "hello world" type test. Thanks to everyone on that issue and in particular @addaleax! This moves some code from shell.js into side .js files, so that we can use it in multiple places. Fixes emscripten-core#6567 but as we add more tests I'm sure we'll find more issues, this just shows basic functionality so far. We may not be able to easily reuse the existing browser tests as they have browser-specific things in them, but we can try.

See emscripten-core#9745 (comment)

nohhhon

node -v
npm -v

nohhhon · 2021-05-23T20:33:40Z

src/library_pthread.js

@@ -432,6 +432,18 @@ var LibraryPThread = {
          worker.onerror = function(e) {
            err('pthread sent an error! ' + e.filename + ':' + e.lineno + ': ' + e.message);


Storage Allocation Method. Hardware is needed.. static Ram. And bios need to be on an external harddrive. .3 disk and 5 readers . With the modifications of each disc except for the center disc need to rotate at a syncrinive rpm in opposite directions this the nodes and threads will be will be twin cousins to the interface OS.. thus the external hardware will see them as one until data is deleted... The electron nodes only take in and recognize hardware attributed firmware ,threads don't recognize any thing except nodes. Environment is semi magnetic before and virtual after... Thus nodes and threads need to be external of an O.S,. [∆π¥] ..Z +Z²=©°![Uploading b5{3c0afaa} (ae8ba507-5d6d-488c-88eb-8b4ffa783c47 id=0x7f0a0v.www.darpa.mil

nohhhon · 2021-05-23T20:34:53Z

src/node_shell_read.js

+    ret = tryParseAsDataURI(filename);
+    if (!ret) {
+#endif
+      if (!nodeFS) nodeFS = require('fs');


src/node_shell_read.js

kripken and others added 22 commits May 11, 2019 13:38

horrible wip [ci skip]

866676b

bad wip exploration [ci skip]

5e80c1f

note the issue [ci skip]

6c427c7

wip [ci skip]

3604163

Merge remote-tracking branch 'origin/incoming' into node-pthreads

14eb654

wip [ci skip]

a90fc1b

Merge remote-tracking branch 'origin/incoming' into node-pthreads

3f07026

more logging [ci skip]

7a3716d

more [ci skip]

257b02b

Merge [ci skip]

5a7731d

Merge remote-tracking branch 'origin/incoming' into node-pthreads

0694d4f

exit pthread properly on node [ci skip]

5a8c86d

frist test

4120e71

TODO [ci skip]

1d85ac0

testing

b6003c2

cleanups

9619833

more cleanup

fb605f0

revert

1e2e871

fix closure

fed61a3

fix

399e9ca

fix

b5245d0

cleanup

43c180a

kripken mentioned this pull request Oct 31, 2019

Support for node.js workers #6567

Closed

addaleax approved these changes Oct 31, 2019

View reviewed changes

kripken mentioned this pull request Oct 31, 2019

binaryen.js wasm-opt commandline tool WebAssembly/binaryen#2411

Open

kripken added 4 commits October 31, 2019 15:15

Merge remote-tracking branch 'origin/incoming' into node-pthreads

cc9c1de

feedback

da9d89e

Nicer warning

ec991f8

More event handling

b6cbb22

kripken requested a review from sbc100 October 31, 2019 22:41

kripken requested a review from dschuff October 31, 2019 22:41

dschuff reviewed Oct 31, 2019

View reviewed changes

kripken added 2 commits October 31, 2019 16:29

feedback

1073fa5

flake8

8354dff

dschuff approved these changes Nov 1, 2019

View reviewed changes

kripken added 4 commits November 1, 2019 13:39

text

fe6edc9

use perf_hooks.performance, and use it for all threads

4858324

Merge remote-tracking branch 'origin/incoming' into node-pthreads

7bf730f

Merge remote-tracking branch 'origin/incoming' into node-pthreads

5c1c6d7

kripken mentioned this pull request Nov 1, 2019

Node Worker pthreads pool reuse #9763

Closed

kripken merged commit 5e3029d into incoming Nov 1, 2019

kripken deleted the node-pthreads branch November 1, 2019 21:29

kripken added a commit that referenced this pull request Nov 1, 2019

Avoid an unnecessary copy in node reads. See #9745 (comment)

0eff776

kripken mentioned this pull request Nov 1, 2019

Avoid an unnecessary copy in node reads. #9764

Merged

woodser mentioned this pull request Nov 3, 2019

shared:ERROR: unexpected error while trying to eval ctors Fatal: ...stopping since could not flatten memory WebAssembly/binaryen#2418

Open

kripken added a commit that referenced this pull request Nov 6, 2019

Avoid an unnecessary copy in node reads. (#9764)

b0eaaa5

See #9745 (comment)

belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020

Avoid an unnecessary copy in node reads. (emscripten-core#9764)

e474466

See emscripten-core#9745 (comment)

sbc100 mentioned this pull request Jan 6, 2021

Ensure Node worker threads are exited gracefully #12963

Merged

nohhhon reviewed May 23, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pthreads support for Node.js #9745

Pthreads support for Node.js #9745

kripken commented Oct 31, 2019 •

edited

Loading

addaleax left a comment

addaleax Oct 31, 2019

kripken Oct 31, 2019

dschuff Oct 31, 2019

kripken Oct 31, 2019

dschuff Nov 1, 2019

dschuff Oct 31, 2019

kripken Oct 31, 2019

abram commented Jan 31, 2020

kripken commented Jan 31, 2020

abram commented Feb 1, 2020

nohhhon left a comment

nohhhon May 23, 2021

nohhhon May 23, 2021

	return nodeFS.readFileSync(filename).toString();
	return nodeFS.readFileSync(filename, 'utf8');

		@@ -0,0 +1,62 @@
		// Copyright 2019 The Emscripten Authors. All rights reserved.

		@@ -432,6 +432,18 @@ var LibraryPThread = {
		worker.onerror = function(e) {
		err('pthread sent an error! ' + e.filename + ':' + e.lineno + ': ' + e.message);

Pthreads support for Node.js #9745

Pthreads support for Node.js #9745

Conversation

kripken commented Oct 31, 2019 • edited Loading

addaleax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abram commented Jan 31, 2020

kripken commented Jan 31, 2020

abram commented Feb 1, 2020

nohhhon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kripken commented Oct 31, 2019 •

edited

Loading